-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Decoder stateful #663
base: master
Are you sure you want to change the base?
Decoder stateful #663
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jen jedna pripominka
self.train_mode, | ||
lambda: tf.transpose(self.train_output_states, [1, 0, 2])[:, :-2], | ||
lambda: tf.transpose( | ||
self.runtime_output_states, [1, 0, 2])[:, :-2]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proc tady zahazujes ty posledni dva prvky? Mozna by to chtelo komentar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dva komenty plus jeden request tady:
Je třeba přejmenovat encoder
u labeleru a taky mu změnit typ v konstruktoru. Třeba na temporal stateful, to se asi hodí nejvíc.
|
||
@tensor | ||
def temporal_states(self) -> tf.Tensor: | ||
return tf.cond( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
přesvědč mě prosím, že to chceme mít závislé na dekodéřím train modu.. Neni reálná situace, kdy chceme labeler trénovat na runtime stavech, případně běhat labeler s trénovacím módem dekodéru? (To druhý asi ne; nicméně pokud mam zafixovanej dekodér a trénuju si labeler, tak self.train_mode
bude nejspíš true, takže se mi muj labeler nakazí exposure biasem od dekodéru, ne?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Představ si, že chceš trénovat multitask překlad a POS tagging na výstupu. Máš POS tagy pro trénovací data. Pak přece nemůžeš nechat dekodér si generovat, co chce, aby labeler nechytil exposure bias. Musíš naopak zařídit, aby ten výstup dekodéru odpovídal tomu, co je v těch zlatých datech, protože jinak nemáš záruku, že stav enkodéru odpovídá tomu, co máš ve zlatých datech.
Kdybys chtěl trénovat ty tagy na runtime stavech, tak bys musel dokázat zjišťovat ty správný tagy dynamicky, podle toho, co se zrovna vydekódovalo, a to není možné.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A jaký slovo tim taggerem labeluješ? Předchozí vydekódovaný nebo zrovna vydekódovaný? Protože jestli zrovna vydekódovaný, tak to je taky něco jinýho než reference.
self.encoder.input_sequence.temporal_states, 2) | ||
dweights_4d = tf.expand_dims( | ||
tf.expand_dims(self.decoding_residual_w, 0), 0) | ||
if hasattr(self.encoder, "input_sequence"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co když je self.encoder
dekodér - nezajímají nás embeddingy slov, co lezou na vstup dekodéru? Asi by to bylo už na větší refactor..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja bych spis cekal, ze te spis zajimaji skryte stavy (resp. to, na zaklade ceho potom predikujes vystupy).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Když ten labeler pracuje nad enkodérem, tak kouká jak na stavy, tak na vstupy. A tadyten if je tu proto, že u dekodéru to padalo, protože nemá vstupy. Ale on je taky svym způsobem má. Plus je samozřejmě ošklivý if
přes hasattr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mohly by nás zajímat, ale znamenalo by to docela dost překopat autoregressive decoder a do toho se mi nechce pouštět. Navíc by to potom nebyla input sequence a bylo potřeba nějak sjednotit interface věcí, co mají embeddingy.
Jen nápad: Co přidat třídu (Nejlepší by bylo mít to jako metodu dekodéru, ale ta nejde zavolat z konfiguráku.) |
Já bych přidal nějakou kouzelnou věc, která by z tenzorů v čase (resp prostoru) udělala temporal (resp spatial) stateful objekty. Takže bychom měli v dekodéru normálně: @tensor
def runtime_logits(self) -> tf.Tensor:
#... pak někde v nějakym helper modulu něco jako def make_temporal_stateful(tensor: tf.Tensor) -> TemporalStateful:
#... a v konfigu: [decoder_logits]
class=make_temporal_stateful
tensor=<decoder.runtime_logits> No a nebo by se ten |
@jindrahelcl Musel by se přidat nějaký Navíc možná chceme mít možnost, aby to bylo závislé na tom train modu, ale to já neumím posoudit. |
Kouzelná věc mi přijde zbytečně složitá, pokud nemáme jinej usecase, než tohle. |
mypy mi říká:
Nevíte někdo, co to je? |
|
Kdežto tady se |
Aha, tak přejmenujeme to u transormera na |
Můžeme
Dne 5. 3. 2018 12:16 odp. napsal uživatel "Jindřich Libovický" <
[email protected]>:
… Aha, tak přejmenujeme to u transormera na model_dimension?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#663 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABwcs_0ilt9qcCXXkmkA2Je4vtYEIl9tks5tbR5ygaJpZM4SZqtw>
.
|
4ff79ac
to
d665a6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, plus bych přidal do testů případ, kdy je labeler stacklej na autoreg. dekodéru.
|
||
Note that when the labeler is stacked on an autoregressive decoder, it | ||
labels the symbol that is currently generated by the decoder, i.e., the | ||
decoder's state has not yet been updated by putting the decoded symbol on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/'s//
Note that when the labeler is stacked on an autoregressive decoder, it | ||
labels the symbol that is currently generated by the decoder, i.e., the | ||
decoder's state has not yet been updated by putting the decoded symbol on | ||
its input. The label is thus the label of a symbol is generated, not the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nesrozumitelná věta
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(ta co začíná The label)
d665a6f
to
bdd7cf3
Compare
Tak, když jsem přidal pořádnej tests, tak to padá na tom, že za runtimu není nafeedovaná |
Zkus TF 1.5 on ten cond to tam může propouštět i když je runtime.. ale kvůli tomu tam jsou ty lambdy, ne? |
ok tak TF 1.6 :-) |
@jlibovicky tohle je taková kravina, že by se dala rebasnout a mergnout, ne? |
Zkus to, ale já mylslim, že to na něčem padalo. |
f535288
to
d0da72f
Compare
@tensor | ||
def temporal_states(self) -> tf.Tensor: | ||
# strip the last symbol which is </s> | ||
return tf.cond( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tohle takhle nefunguje. tf.cond
potřebuje všechny placeholdery, i když se nakonec vyhodnotí opačně.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jo, to už jsem jednou věděl. Myslím, že se to mělo spravit v nějaké další verzi TF. Jak jsme na tom s kompatibilitou s novými verzemi?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nejake pripominky
return tf.reduce_sum(weighted_loss) | ||
min_time = tf.minimum(tf.shape(self.train_targets)[1], | ||
tf.shape(self.logits)[1]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skutecne tu chceme vybirat minimum behem trenovani. Nehrozi treba, ze se sit nauci spravne generovat (dekoderovou vrstvou) napr. pouze sekvence delky 1, ktere ale bude spravne labelovat.
Nemely by se logits "paddovat" na train_targets length, nebo nejakym jinym zpusobem penalizovat kratsi sekvence?
logits=self.logits[:, :min_time], | ||
targets=self.train_targets[:, :min_time], | ||
weights=self.input_sequence.temporal_mask[:, :min_time]) | ||
# pylint: enable=unsubscriptable-object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je skutecne v poradku tady nahradit sumu pres logity meanem? Minimalne z hlediska zpetne kompatibility (trenovacich hyperparametru) to uplne koser nebude.
# of mypy not being able to handle the tf.Tensor type. | ||
assert self.encoder_states is not None | ||
|
||
self.model_dimension = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nebylo by lepsi misto prejmenovavani radej presunout radky 130-136 do overridnute property dimension? Takhle vznika zmatek v kodu
This PR make autoregressive decoder instance of
TemporalStateful
such we can stack a sequence labeler on top of it.